feat: identity to addresses #315
Conversation
Hanssen0
commented
Sep 17, 2025
Hanssen0
commented
- I have read the Contributing Guidelines
🦋 Changeset detectedLatest commit: 32b18a7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for liveccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for docsccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for apiccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for appccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Hanssen0
force-pushed
the
feat-identity-address
branch
3 times, most recently
from
a12b9a9 to
a48e376
Compare
|
/canary |
|
🚀 Canary version published successfully! View workflow run The following packages have been published to npm:
|
Hanssen0
force-pushed
the
feat-identity-address
branch
from
a48e376 to
bc5a1a7
Compare
|
/canary |
|
🚀 Canary version published successfully! View workflow run The following packages have been published to npm:
|
Hanssen0
force-pushed
the
feat-identity-address
branch
from
bc5a1a7 to
32b18a7
Compare
|
/canary |
|
🚀 Canary version published successfully! View workflow run The following packages have been published to npm:
|
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to create a Signer from a signature and includes the address in the JoyID identity. The changes are logical, but there are a few areas that could be improved for robustness and maintainability. Specifically, there are unsafe uses of JSON.parse that could lead to runtime errors, and some hardcoded URLs that should be extracted as constants. I've left specific comments on these points.
| const { address, publicKey, keyType } = JSON.parse(identity) as { | ||
| address: string; | ||
| publicKey: string; | ||
| keyType: string; | ||
| keyType: CredentialKeyType; | ||
| }; | ||
| const signatureObj = JSON.parse(signature) as { | ||
| alg: SigningAlg; | ||
| signature: string; | ||
| message: string; | ||
| }; |
There was a problem hiding this comment.
Using JSON.parse with type assertions (as) is not type-safe. If the identity or signature strings are malformed or don't match the expected structure, this will throw an unhandled exception at runtime, which could crash the application. It's recommended to wrap these parsing operations in a try...catch block and validate the parsed objects to handle potential errors gracefully.
For example:
let identityObj: { address: string; publicKey: string; keyType: CredentialKeyType; };
let signatureObj: { alg: SigningAlg; signature: string; message: string; };
try {
identityObj = JSON.parse(identity);
signatureObj = JSON.parse(signature);
} catch {
return false;
}
const { address, publicKey, keyType } = identityObj;
if (!address || !publicKey || !keyType || !signatureObj.alg || !signatureObj.signature) {
return false;
}| const { address } = JSON.parse(signature.identity) as { | ||
| address: string; | ||
| }; |
There was a problem hiding this comment.
Using JSON.parse with a type assertion is unsafe. If signature.identity is not valid JSON or doesn't contain an address property, this will cause a runtime error. It's important to handle this case to prevent the application from crashing.
Consider using a try...catch block and validating the parsed object:
case SignerSignType.JoyId: {
let address: string;
try {
const identity = JSON.parse(signature.identity);
address = identity.address;
} catch (e) {
throw new Error(`Failed to parse JoyId identity: ${e}`);
}
if (!address) {
throw new Error("Missing address in JoyId identity");
}
return new SignerCkbScriptReadonly(
client,
(await Address.fromString(address, client)).script,
);
}| const registry = address.startsWith("ckb") | ||
| ? "https://api.joy.id/api/v1/" | ||
| : "https://api.testnet.joyid.dev/api/v1/"; |
There was a problem hiding this comment.
Hardcoding URLs directly within the function makes the code harder to maintain and test. If these URLs change, you'll have to find and replace them in the code. It would be better to extract these URLs into named constants at the module level. This improves readability and centralizes configuration.
| switch (signature.signType) { | ||
| case SignerSignType.EvmPersonal: | ||
| return new SignerEvmAddressReadonly(client, signature.identity); | ||
| case SignerSignType.BtcEcdsa: | ||
| return new SignerBtcPublicKeyReadonly(client, "", signature.identity); | ||
| case SignerSignType.JoyId: { | ||
| const { address } = JSON.parse(signature.identity) as { | ||
| address: string; | ||
| }; | ||
| return new SignerCkbScriptReadonly( | ||
| client, | ||
| (await Address.fromString(address, client)).script, | ||
| ); | ||
| } | ||
| case SignerSignType.NostrEvent: | ||
| return new SignerNostrPublicKeyReadonly(client, signature.identity); | ||
| case SignerSignType.CkbSecp256k1: | ||
| return new SignerCkbPublicKey(client, signature.identity); | ||
| case SignerSignType.DogeEcdsa: | ||
| return new SignerDogeAddressReadonly(client, signature.identity); | ||
| case SignerSignType.Unknown: | ||
| throw new Error("Unknown signer sign type"); | ||
| } |
There was a problem hiding this comment.
The switch statement is missing a default case. If a new SignerSignType enum member is added in the future, it will fall through the switch, signer will be undefined, and signer.getAddresses() on line 59 will throw a TypeError. To make this more robust, you should add a default case to handle any unexpected signType values by throwing an error.
For example:
switch (signature.signType) {
// ... other cases
default:
throw new Error(`Unsupported sign type: ${signature.signType as string}`);
}1 participant
